Created a centralized please command #1537
Conversation
|
I was able to test this in VSCode and jet brains on the mac but I was unable to test the other implementations to make sure they're working. I could use a hand with testing on other platforms to make sure these things still work, though the implementations appear correct to me right now. |
There was a problem hiding this comment.
Makes sense to use a tag for this to avoid "promising what we can't deliver". With the exception of the tiny nit I mentioned, the only other thing I can think that might is whether we'd like to continue sending an empty string for "please" by itself or define two voice commands and an action with an optional argument. I don't feel strongly one way or the other.
apps/arc/arc.py
Outdated
| @@ -20,6 +20,11 @@ def tab_close_wrapper(): | |||
| actions.sleep("180ms") | |||
| actions.app.tab_close() | |||
|
|
|||
| def please_command(command: str): | |||
There was a problem hiding this comment.
We probably want to call this command_search instead. Also the argument should probably be optional.
There was a problem hiding this comment.
I tried to do this but I'm not sure if I did it right. Please check my work-thank you!
There was a problem hiding this comment.
I guess there is a discussion if we want to default to an empty string or None.
Also I personally feel that if the argument is optional we should do an if statement an only do sleep and insert if we actually have an argument.
https://github.com/AndreasArvidsson/andreas-talon/blob/114911424dc568d833b865f64f8607ac83275a6d/apps/vscode/vscode_git.py#L63-L67
There was a problem hiding this comment.
When calling the action from talon script, you still need to pass the argument (I.e. arg or ""), right? Otherwise you'll get an error, I think. Unless there's some way I don't know about.
There was a problem hiding this comment.
No, not of its optional. You just need to do it that way I linked above, but you need to call it from a separate command. (arg) doesn't work if arg is optional. You call it without any argument.
There was a problem hiding this comment.
I went through and added the conditions. Do you think I should add the separate command? I wasn't sure about that.
To avoid divergence in the grammar for please commands, it is useful to define a centralized tag and allow it to be overridden in command implementations.
- It looks like the formatting was more than just adding a comma ; this should be cleaner now
jaresty
force-pushed
the
centralized-please-command
branch
from
3750d85
to
1a17a4c
Compare
- Only insert text if it was searched for
To avoid divergence in the grammar for please commands, it is useful to define a centralized tag and allow it to be overridden in command implementations.
Addresses #1536